Skip to content

Ollama format 修正 + web データ取得フック共通化・サイドバー配色修正#423

Merged
yusuke0610 merged 5 commits into
mainfrom
fix/ollama-format-portable-schema
Jun 28, 2026
Merged

Ollama format 修正 + web データ取得フック共通化・サイドバー配色修正#423
yusuke0610 merged 5 commits into
mainfrom
fix/ollama-format-portable-schema

Conversation

@yusuke0610

@yusuke0610 yusuke0610 commented Jun 28, 2026

Copy link
Copy Markdown
Owner

概要

このブランチには 3 つの変更 が含まれます(うち Ollama 修正は既存コミット)。

1. fix(agent): Ollama の format に移植スキーマを渡し文法変換失敗を解消 (fa75ff1)

  • 既存コミット。Ollama の format に移植スキーマを渡し、文法変換失敗を解消。

2. refactor(web): データ取得フックの共通化とフォーム制御の分離

  • enabled-gated fetch の定型(loading/error/refresh + seq ガード)を新規 useAsyncResource に集約し、useCreditBalance / useAgentUsageSummary / useModelRates / useBillingPage を載せ替え。これまで残高フックだけが持っていた競合上書き防止(seq ガード)を全フックへ波及
  • CareerResumeForm のバリデーション/フォーカス/送信分岐を useCareerFormValidationFocus に切り出し、本体を UI 構成 + フック配線に純化。
  • ドリフトしていた career サンプル fixture を test/factories/careerForm.ts に一本化。
  • AgentChatWidget のチャット履歴を index → 安定 id キー化。useAgentChat.test の反復 arrange を sendChat ヘルパへ集約。
  • 新規テスト 2 本追加(useAsyncResource の enabled/seq ガード、useCareerFormValidationFocus の隠れフィールド自動展開など)。

3. fix(web): ライトモードでサイドバーのモデル/残高バッジが読めない配色を修正

  • サイドバー背景は両テーマで暗色だが、AgentModelBadge / CreditBalanceBadge が本文用の --text-primary / --text-secondary(ライトでは暗色)を使っていたため暗背景に暗文字で読めなかった。サイドバー専用の --sidebar-text / --sidebar-title に差し替え。

検証

  • make ci: pass(backend lint/test + web lint/test/build)
  • web unit: 404 件(vitest 400 + node:test 4)全 green
  • jscpd: fetch フックの非テスト重複は 0 件に収束(残りは icons / setAtPath の許容分のみ)
  • E2E: 未実行(新規ページ/ルート・認証/ナビ/レイアウト変更に非該当の内部リファクタ + CSS 修正のため)

補足

  • レビュー観点の詳細は report/WEB_report_20260628_1533.md / report/WEB_pr_20260628_1602.md を参照。
  • 公開 API(各フックの戻り値)は不変。挙動は既存テスト + 新規ユニットテストで保護。

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Improved career resume “save-time” handling with more targeted validation feedback and automatic editor/modals when fields are invalid.
    • More consistent async loading/error handling across credit, billing, usage, and model rate displays.
  • Bug Fixes
    • Improved chat rendering stability by using stable identifiers for message list entries.
  • Style
    • Refreshed sidebar badge and credit/billing badge colors for visual consistency.
  • Tests
    • Added/updated unit tests covering chat keys, validation/focus behavior, async resource lifecycle, and Ollama schema compatibility.

yusuke0610 and others added 3 commits June 28, 2026 15:14
ローカル Ollama(LLM_LOCAL_OLLAMA=1)経路で /api/chat が
"Failed to initialize samplers: failed to parse grammar" の 400 を返し、
Agent チャットが 502 になっていた。原因は format に渡す JSON Schema の
maxLength / maxItems を llama.cpp の GBNF 文法変換が解釈できないこと
(oneOf は無罪。実機 curl で maxLength/maxItems 除去後は 200 を確認)。

- ollama_client: format に to_portable_schema(..., drop_additional_properties=False)
  を渡す(maxLength/maxItems 除去・oneOf→enum 平坦化)。Gemini/OpenAI と同じ移植経路に揃える
- output_schema: 「Ollama は maxLength を助言扱い/生スキーマを使う」誤コメントを実態へ修正
- test_llm_clients: format に oneOf/maxLength/maxItems が残らないことの回帰テストを追加

文字数上限の実強制は従来どおり _parse_response が担う(二重防衛)。
Anthropic/Gemini/OpenAI 経路は無変更。make lint-backend / make test-backend(633 passed) green。

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- enabled-gated fetch の定型(loading/error/refresh + seq ガード)を useAsyncResource に
  集約し、useCreditBalance / useAgentUsageSummary / useModelRates / useBillingPage を載せ替え
  (競合上書き防止を全フックへ波及)
- CareerResumeForm のバリデーション/フォーカス/送信分岐を useCareerFormValidationFocus に
  切り出し、本体を UI 構成に純化
- career サンプル fixture を test/factories/careerForm.ts に一本化
- AgentChatWidget のチャット履歴を安定 id キー化、useAgentChat.test の反復 arrange を
  sendChat ヘルパへ集約

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
サイドバー背景は両テーマで暗色だが、AgentModelBadge / CreditBalanceBadge が本文用の
--text-primary / --text-secondary(ライトでは暗色)を使っていたため暗背景に暗文字で
読めなかった。サイドバー専用の --sidebar-text / --sidebar-title に差し替える。

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 28, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@yusuke0610, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 20 minutes and 48 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 5dc6689e-1e19-4cba-95e7-5e4fe799ac7f

📥 Commits

Reviewing files that changed from the base of the PR and between ffc67bb and 68be942.

📒 Files selected for processing (1)
  • web/src/hooks/useAsyncResource.test.ts
📝 Walkthrough

Walkthrough

This PR adds useAsyncResource and migrates several data hooks to it, extracts career form validation and submit flow into useCareerFormValidationFocus, gives chat entries stable ids for rendering, updates Ollama schema handling, switches sidebar badge colors, and adds a shared career form test factory.

Changes

Ollama Portable Schema

Layer / File(s) Summary
Portable schema transformation in OllamaClient
backend/app/services/agent/output_schema.py, backend/app/services/agent/llm/ollama_client.py, backend/tests/test_llm_clients.py
to_portable_schema is imported and applied to output_schema before sending the format payload in OllamaClient.generate. Related docs clarify provider-specific schema handling, and a regression test checks the outgoing payload no longer contains maxLength, maxItems, or oneOf.

Frontend: Async Resource Hook, Validation Focus Hook, Chat IDs, CSS, and Test Factories

Layer / File(s) Summary
useAsyncResource hook and consumer migrations
web/src/hooks/useAsyncResource.ts, web/src/hooks/useAsyncResource.test.ts, web/src/hooks/useCreditBalance.ts, web/src/hooks/useModelRates.ts, web/src/hooks/useAgentUsageSummary.ts, web/src/hooks/useBillingPage.ts
useAsyncResource adds enabled-controlled fetch state, request-sequence guarding, error mapping, and refresh(). useCreditBalance, useModelRates, useAgentUsageSummary, and useBillingPage now use it, and the new test suite covers fetch enablement, rejection handling, stale responses, and enable toggling.
useCareerFormValidationFocus hook and CareerResumeForm refactor
web/src/hooks/career/useCareerFormValidationFocus.ts, web/src/hooks/career/useCareerFormValidationFocus.test.ts, web/src/components/forms/CareerResumeForm.tsx
The new hook manages validation errors, focus targets, markdown-field opening, and authenticated versus unauthenticated submit branching. CareerResumeForm removes its inline validation and submit logic and delegates to the hook. Tests cover submit branches, markdown focus, save confirmation, and clearing state on edit.
Stable id on AgentChatEntry and AgentChatWidget key fix
web/src/hooks/career/useAgentChat.ts, web/src/hooks/career/useAgentChat.test.ts, web/src/components/forms/AgentChatWidget.tsx
AgentChatEntry now includes an id assigned from a monotonic counter in useAgentChat, and AgentChatWidget uses that id as the React key. The hook tests add a unique-id assertion and switch repeated submissions to a shared sendChat helper.
Shared careerForm test factory
web/src/test/factories/careerForm.ts, web/src/hooks/career/useCareerDirty.test.ts, web/src/utils/careerDiff.test.ts
A new buildSampleCareerForm factory constructs a reusable deep-cloned CareerFormState, and the existing test files replace their inline builders with it.
Sidebar CSS variable updates
web/src/components/agent/AgentModelBadge.module.css, web/src/components/billing/CreditBalanceBadge.module.css
The badge styles switch from generic text variables to sidebar-specific text and title variables, with comments added around the sidebar color choice.

Sequence Diagram(s)

sequenceDiagram
  participant Consumer
  participant useAsyncResource
  participant API

  Consumer->>useAsyncResource: configure(fetcher, enabled, initialData, fallbackMessage)
  useAsyncResource->>API: fetcher()
  alt success
    API-->>useAsyncResource: data
    useAsyncResource->>Consumer: data, loading=false, error=null
  else failure
    API-->>useAsyncResource: error
    useAsyncResource->>Consumer: error fallback or message
  end
  Consumer->>useAsyncResource: refresh()
  useAsyncResource->>API: fetcher() with requestSeq guard
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • yusuke0610/devforge#422: Updates ollama_client.py to send format=to_portable_schema(...) and adjusts output_schema.py docs, directly overlapping with the backend schema changes in this PR.

Poem

🐰 Hop hop, the schemas transform with grace,
Async hooks now share their tidy place.
Stable IDs blink in the chat line’s glow,
And sidebar colors softly flow.
The form now hops with less to mend—
A neater forge from end to end.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly reflects the three main change areas: Ollama format fix, web data-fetch hook refactor, and sidebar color updates.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/ollama-format-portable-schema

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@web/src/hooks/career/useCareerFormValidationFocus.ts`:
- Around line 104-114: The guest-submit branch in useCareerFormValidationFocus
currently relies on a pending effect to save the latest form state before login,
which can miss the last edit. Update this hook to accept a synchronous
persistDraft(form) callback and call it immediately before requestLogin() in the
isAuthenticated false path, using the existing validation and focus-reset flow
unchanged.

In `@web/src/hooks/useAsyncResource.ts`:
- Around line 60-64: The enabled-gated flow in useAsyncResource still allows an
in-flight refresh started by refresh() to update state after enabled flips to
false because the true-to-false transition never invalidates requestSeqRef.
Update the useEffect that watches enabled and refresh so that disabling the hook
also bumps requestSeqRef (or otherwise marks the current request stale) before
any pending async work can commit state, keeping the existing refresh() and
requestSeqRef logic aligned with the enabled contract.
- Around line 51-54: The error handling in useAsyncResource currently uses
Error.message even when it is an empty string, which bypasses the intended
fallback behavior. Update the catch block in useAsyncResource so that when the
caught value is an Error with a blank message, it uses fallbackMessage instead
of storing an empty error string; keep the existing seq and
requestSeqRef.current guard intact while adjusting the setError logic.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 7524683b-b07c-4950-b8a4-412c6b9a5e92

📥 Commits

Reviewing files that changed from the base of the PR and between 5c5e5df and 0ba470c.

📒 Files selected for processing (20)
  • backend/app/services/agent/llm/ollama_client.py
  • backend/app/services/agent/output_schema.py
  • backend/tests/test_llm_clients.py
  • web/src/components/agent/AgentModelBadge.module.css
  • web/src/components/billing/CreditBalanceBadge.module.css
  • web/src/components/forms/AgentChatWidget.tsx
  • web/src/components/forms/CareerResumeForm.tsx
  • web/src/hooks/career/useAgentChat.test.ts
  • web/src/hooks/career/useAgentChat.ts
  • web/src/hooks/career/useCareerDirty.test.ts
  • web/src/hooks/career/useCareerFormValidationFocus.test.ts
  • web/src/hooks/career/useCareerFormValidationFocus.ts
  • web/src/hooks/useAgentUsageSummary.ts
  • web/src/hooks/useAsyncResource.test.ts
  • web/src/hooks/useAsyncResource.ts
  • web/src/hooks/useBillingPage.ts
  • web/src/hooks/useCreditBalance.ts
  • web/src/hooks/useModelRates.ts
  • web/src/test/factories/careerForm.ts
  • web/src/utils/careerDiff.test.ts

Comment thread web/src/hooks/career/useCareerFormValidationFocus.ts
Comment thread web/src/hooks/useAsyncResource.ts
Comment thread web/src/hooks/useAsyncResource.ts
- useAsyncResource: enabled が true→false に変わった瞬間に requestSeqRef を進め、
  進行中リクエストの後追い応答が state を上書きしないようにする(無効中は更新しない契約を遵守)
- useAsyncResource: Error でも message が空文字なら fallback を使う
- useCareerFormValidationFocus: ゲストのログイン遷移直前に persistDraft で現在のフォームを
  同期退避し、effect 未反映による最後の入力の取りこぼしを防ぐ
- 上記を保護するユニットテストを追加

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@web/src/hooks/useAsyncResource.test.ts`:
- Around line 97-125: The test for useAsyncResource should verify that the
enabled=true path actually triggers the fetcher before toggling enabled to
false, otherwise it can pass even if auto-fetch is broken. In
useAsyncResource.test.ts, add an assertion on the fetcher mock (or another
explicit check tied to the renderHook setup) after the initial render and before
rerendering, so the test confirms the fetcher was called and the in-flight
request is what gets invalidated when enabled changes.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: a244a15f-09c6-40a4-9337-94d7dd1eba56

📥 Commits

Reviewing files that changed from the base of the PR and between 0ba470c and ffc67bb.

📒 Files selected for processing (5)
  • web/src/components/forms/CareerResumeForm.tsx
  • web/src/hooks/career/useCareerFormValidationFocus.test.ts
  • web/src/hooks/career/useCareerFormValidationFocus.ts
  • web/src/hooks/useAsyncResource.test.ts
  • web/src/hooks/useAsyncResource.ts
🚧 Files skipped from review as they are similar to previous changes (4)
  • web/src/hooks/useAsyncResource.ts
  • web/src/hooks/career/useCareerFormValidationFocus.ts
  • web/src/components/forms/CareerResumeForm.tsx
  • web/src/hooks/career/useCareerFormValidationFocus.test.ts

Comment thread web/src/hooks/useAsyncResource.test.ts
loading 初期値が enabled 由来で true のため、自動 fetch が退行してもテストが通ってしまう
弱点を修正。enabled→false に切り替える前に fetcher の呼び出しを waitFor で確認し、
「進行中リクエストが無効化される」ことを確実に検証する。

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@yusuke0610
yusuke0610 merged commit 2cddc79 into main Jun 28, 2026
17 checks passed
@yusuke0610
yusuke0610 deleted the fix/ollama-format-portable-schema branch July 20, 2026 12:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant